gh-144650: support Vectorcall for _decimal.Decimal's - #158207

#157968 may help with the build failures (changes how the assert works which should make it better across module boundaries)

Thanks. The Decimal is only possible candidate being a heap type in the stdlib. That was worked before in skirpichev#18

👍 hopefully will get more of these landed soon :). str and list still need some special work for stable performance over the change which I'm still root causing (likely compiler inlining vs. existing hand inlining). If there are ways I can help enable / particular edge cases you find let me know. Otherwise planning to make PRs as I have time + performance results for existing cases across the codebase (int, float, str, list, bytes, bytearray, bool, filter, set, frozenset are on my radar).

CC @vstinner, does it make sense for you?

Edit: N.B., this depends on #157968.

I think it still worth it, assuming micro-benchmarks. It's for free, anyway.

But it's on top of #157968. If you are ok, I'll reopen this one that pr (or some its alternative) will be merged.

See also #73487 which converted many Decimal methods to vectorcall using Argument Clinic.

#158207 (comment) were posted 1 month ago, but the PR changed in the meanwhile. Would it be possible to re-run benchmarks on the latest PR version?

In fact, you can see that benchmarks already updated. Not so much, as before. #156034 shows better numbers.

Now included telco benchmark.

Is it useful to mark the function as "inline"? Does it impact performance?

This is something that was suggested in #160115.

You can compare my previous benchmark run. IMO, the difference looks more or less as a noise:
#160115 (comment)

Not sure if this worth reversion. That function used only once, anyway.

I don't mind the inline keyword, it's a compiler hint that is largely ignored (which is why forceinline exists..). Compilers decided people perf tested and added then never maintained ><. In this case PyDecType_FromObjectExact inlines into the _new_impl with or without it in the experimentation I've done. It doesn't inline into the vectorcall handler because there are two callers of _new_impl and the compiler decides not to proceed.

Sorry, @cmaloney, it seems that your suggestion is not clear for me.

I have no preference on inline keyword or not. In this case i did some assembly looking and without the keyword it does get inlined to the _new_impl.

In fact, you can see that benchmarks already updated. Not so much, as before. #156034 shows better numbers.

#156034 (comment) was posted in August. Python changed since August, so I prefer to not rely on old benchmark numbers.

It's unclear to me if #160115 (comment) include the "static inline" change or not.

I would be easier for me if you can post benchmark results in this PR rather than pointing to other issues/PRs.

Can you please re-run benchmarks on the latest PR version, since you just modified it?

Can you please re-run benchmarks on the latest PR version, since you just modified it?

Ok, done. (Same setup: default configure options, gcc 14.2.)

decimal_pi: Mean +- std dev: [ref] 2.00 sec +- 0.01 sec -> [patch] 1.97 sec +- 0.01 sec: 1.01x faster

Benchmark hidden because not significant (2): decimal_factorial, telco

Geometric mean: 1.00x faster

Decimal(0): Mean +- std dev: [ref-micro] 563 ns +- 11 ns -> [patch-micro] 510 ns +- 10 ns: 1.10x faster
Decimal(123): Mean +- std dev: [ref-micro] 560 ns +- 11 ns -> [patch-micro] 509 ns +- 10 ns: 1.10x faster
Decimal('123'): Mean +- std dev: [ref-micro] 793 ns +- 11 ns -> [patch-micro] 727 ns +- 13 ns: 1.09x faster

Geometric mean: 1.10x faster

decimal_pi, decimal_factorial and telco benchmark results are a little bit disappointing. (Basically, there is no impact on performance, just a minor "1.01x faster" which could also just be noise in the results).

Decimal(0): Mean +- std dev: [ref-micro] 563 ns +- 11 ns -> [patch-micro] 510 ns +- 10 ns: 1.10x faster
Decimal(123): Mean +- std dev: [ref-micro] 560 ns +- 11 ns -> [patch-micro] 509 ns +- 10 ns: 1.10x faster
Decimal('123'): Mean +- std dev: [ref-micro] 793 ns +- 11 ns -> [patch-micro] 727 ns +- 13 ns: 1.09x faster
Geometric mean: 1.10x faster

1.10x faster on a micro-benchmark is borderline interesting, but I think that adding @vectorcall has a very limited impact on maintenance, so it's worth it to make this change.

I tested a sub-type: since tp_vectorcall is not inherited by sub-types, we are good (dec_vectorcall() is not used, that's ok):

import _decimal

class MyDecimal(_decimal.Decimal):
    pass

x = MyDecimal(123)